Skip to content

Add graphs/frequent_pattern_graph_miner.py#1866

Merged
cclauss merged 37 commits into
TheAlgorithms:masterfrom
siva1098:master
May 7, 2020
Merged

Add graphs/frequent_pattern_graph_miner.py#1866
cclauss merged 37 commits into
TheAlgorithms:masterfrom
siva1098:master

Conversation

@siva1098

@siva1098 siva1098 commented Apr 15, 2020

Copy link
Copy Markdown
Contributor

Describe your change:

  • Add an algorithm?
  • Fix a bug or typo in an existing algorithm?
  • Documentation change?

Checklist:

  • I have read CONTRIBUTING.md.
  • This pull request is all my own work -- I have not plagiarized.
  • I know that pull requests will not be merged if they fail the automated tests.
  • This PR only changes one algorithm file. To ease review, please open separate PRs for separate algorithms.
  • All new Python files are placed inside an existing directory.
  • All filenames are in all lowercase characters with no spaces or dashes.
  • All functions and variable names follow Python naming conventions.
  • All function parameters and return values are annotated with Python type hints.
  • All functions have doctests that pass the automated testing.
  • All new algorithms have a URL in its comments that points to Wikipedia or other similar explanation.
  • If this pull request resolves one or more open issues then the commit message contains Fixes: #{$ISSUE_NO}.

@cclauss cclauss left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your contribution. It needs some changes to meet our standards.

Comment thread graphs/frequent_pattern_graph_miner.py Outdated
Comment thread graphs/frequent_pattern_graph_miner.py Outdated
Comment thread graphs/frequent_pattern_graph_miner.py Outdated
Comment thread graphs/frequent_pattern_graph_miner.py Outdated
Comment thread graphs/frequent_pattern_graph_miner.py Outdated
Comment thread graphs/frequent_pattern_graph_miner.py Outdated
Comment thread graphs/frequent_pattern_graph_miner.py Outdated
Comment thread graphs/frequent_pattern_graph_miner.py Outdated
Comment thread graphs/frequent_pattern_graph_miner.py Outdated
Comment thread graphs/frequent_pattern_graph_miner.py Outdated
Comment thread graphs/frequent_pattern_graph_miner.py Outdated
Comment thread graphs/frequent_pattern_graph_miner.py Outdated
Comment thread graphs/frequent_pattern_graph_miner.py Outdated
Comment thread graphs/frequent_pattern_graph_miner.py Outdated
siva1098 and others added 3 commits April 15, 2020 16:24
Co-Authored-By: Christian Clauss <cclauss@me.com>
Co-Authored-By: Christian Clauss <cclauss@me.com>
Comment thread graphs/frequent_pattern_graph_miner.py
Comment thread graphs/frequent_pattern_graph_miner.py Outdated
Comment thread graphs/frequent_pattern_graph_miner.py Outdated
Comment thread graphs/frequent_pattern_graph_miner.py Outdated
Comment thread graphs/frequent_pattern_graph_miner.py Outdated
Comment thread graphs/frequent_pattern_graph_miner.py Outdated
Comment thread graphs/frequent_pattern_graph_miner.py Outdated
Comment thread graphs/frequent_pattern_graph_miner.py Outdated
Comment thread graphs/frequent_pattern_graph_miner.py Outdated
siva1098 and others added 4 commits April 18, 2020 12:03
Co-Authored-By: Christian Clauss <cclauss@me.com>
Co-Authored-By: Christian Clauss <cclauss@me.com>
Co-Authored-By: Christian Clauss <cclauss@me.com>
Co-Authored-By: Christian Clauss <cclauss@me.com>
Comment thread graphs/frequent_pattern_graph_miner.py Outdated
Co-Authored-By: Christian Clauss <cclauss@me.com>
@cclauss

cclauss commented Apr 18, 2020

Copy link
Copy Markdown
Member

Nice! Test are passing again. (LGTM can be ignored)

Co-Authored-By: Christian Clauss <cclauss@me.com>
@cclauss

cclauss commented Apr 18, 2020

Copy link
Copy Markdown
Member

OK. Now we appear in the Travis tests

Now please remove the second test that was designed to fail.
https://travis-ci.com/github/TheAlgorithms/Python/builds/160822861#L1258

Comment thread graphs/frequent_pattern_graph_miner.py Outdated
Comment thread graphs/frequent_pattern_graph_miner.py Outdated
Comment thread graphs/frequent_pattern_graph_miner.py Outdated
@cclauss

cclauss commented Apr 21, 2020

Copy link
Copy Markdown
Member

https://travis-ci.com/github/TheAlgorithms/Python/builds/161205503#L1270

You can run these tests on your own computer with the command:
python3 -m doctest -v frequent_pattern_graph_miner.py

Comment thread graphs/frequent_pattern_graph_miner.py Outdated
@cclauss cclauss changed the title Add files via upload Add graphs/frequent_pattern_graph_miner.py Apr 21, 2020
Comment thread graphs/frequent_pattern_graph_miner.py Outdated
@siva1098

Copy link
Copy Markdown
Contributor Author

All changes have made.
Pls merge the file.

@siva1098 siva1098 requested a review from cclauss April 24, 2020 13:12
@cclauss

cclauss commented Apr 24, 2020

Copy link
Copy Markdown
Member

Could you please add doctests to at least some of these?

  • get_frequency_table()
  • get_nodes()
  • get_cluster()
  • get_support()
  • construct_graph()
  • freq_subgraphs_edge_list()
  • preprocess()

@siva1098

Copy link
Copy Markdown
Contributor Author

Could you please add doctests to at least some of these?

  • get_frequency_table()
  • get_nodes()
  • get_cluster()
  • get_support()
  • construct_graph()
  • freq_subgraphs_edge_list()
  • preprocess()

Comment thread graphs/frequent_pattern_graph_miner.py Outdated
@@ -100,7 +100,7 @@ def get_support(cluster :Dict[int,Dict[str,List[str]]])-> List[float]:
'''
Returns support
>>> get_support({5: {'11111': ['ab', 'ac', 'df', 'bd', 'bc']}, 4: {'11101': ['ef', 'eg', 'de', 'fg'], '11011': ['cd']}, 3: {'11001': ['ad'], '10101': ['dg']}, 2: {'10010': ['dh', 'bh'], '11000': ['be'], '10100': ['gh'], '10001': ['ce']}, 1: {'00100': ['fh', 'eh'], '10000': ['hi']}})

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lines in Python should be 88 characters max. Lines that can not be read in GitHub without left/right scrolling (127 characters) need to be wrapped for readability.

@cclauss

cclauss commented Apr 25, 2020

Copy link
Copy Markdown
Member

I will review tonight and then I think then we can land this one. Thanks for your persistence!

@cclauss cclauss added the tests are failing Do not merge until tests pass label Apr 29, 2020
@siva1098 siva1098 requested a review from cclauss May 1, 2020 17:21
@cclauss cclauss removed the tests are failing Do not merge until tests pass label May 1, 2020
@siva1098

siva1098 commented May 4, 2020

Copy link
Copy Markdown
Contributor Author

can you pls merge the file?

@cclauss cclauss added the tests are failing Do not merge until tests pass label May 7, 2020
@cclauss cclauss merged commit 7469fb6 into TheAlgorithms:master May 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

tests are failing Do not merge until tests pass

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants